-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-45335: Add note to sqlite3
docs about "timestamp" converter
#29200
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). CLA MissingOur records indicate the following people have not signed the CLA: For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. If you have recently signed the CLA, please wait at least one business day You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
Doc/library/sqlite3.rst
Outdated
@@ -1049,6 +1049,17 @@ If a timestamp stored in SQLite has a fractional part longer than 6 | |||
numbers, its value will be truncated to microsecond precision by the | |||
timestamp converter. | |||
|
|||
.. warning:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We avoid using warnings in documentation. Instead we describe behavior using affirmative tone.
Doc/library/sqlite3.rst
Outdated
.. warning:: | ||
|
||
The "timestamp" converter ignores UTC offsets in the database and always | ||
returns a naive :class:`datetime.datetime` object, so if you read a timestamp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outside of tutorials and howtos, we avoid addressing the reader as "you", instead using a style where the "you" is implied. Instead of saying:
If you want X, do Y.
simply say:
To achieve X, do Y.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I'd drop that part of the sentence. The first part already says that UTC offsets are ignored; no need to exemplify it. How about something like this:
The "timestamp" converter ignores UTC offsets in the database and always
returns a naive :class:`datetime.datetime` object. To preserve UTC offsets
in timestamps, either leave converters disabled, or register offset-aware
converters with :func:`register_converter`.
Doc/library/sqlite3.rst
Outdated
from the database with converters enabled and then write it back, any UTC | ||
offset will be lost. | ||
|
||
If you need to preserve UTC offsets in timestamps, then either leave |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, instead of if..then
just say "To preserve... either... or". This way you avoid the explicit "you".
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
I have made the requested changes; please review again. Thank you for the review. I have used Erlend's wording with minor changes. |
Thanks for making the requested changes! @ambv: please review the changes made to this pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
Should I backport this to older versions of Python as well? Is there documentation on how to do that? |
Thanks for the reminder! I've added backport labels for @miss-islington :) The devguide has a section about backporting. |
Thanks! Hopefully it can be backported automatically, but if not I'll check out the devguide. |
…thonGH-29200) (cherry picked from commit 3877fc0) Co-authored-by: Ian Fisher <[email protected]>
GH-29319 is a backport of this pull request to the 3.10 branch. |
…thonGH-29200) (cherry picked from commit 3877fc0) Co-authored-by: Ian Fisher <[email protected]>
GH-29320 is a backport of this pull request to the 3.9 branch. |
…-29200) (GH-29319) (cherry picked from commit 3877fc0) Co-authored-by: Ian Fisher <[email protected]>
…-29200) (GH-29320) (cherry picked from commit 3877fc0) Co-authored-by: Ian Fisher <[email protected]>
See https://discuss.python.org/t/fixing-sqlite-timestamp-converter-to-handle-utc-offsets/10985 for further discussion of this issue.
https://bugs.python.org/issue45335